new(ci): use `zig` compiler instead of relying on centos7. #3307

FedeDP · 2024-08-28T07:59:48Z

What type of PR is this?

/kind cleanup

Any specific area of the project related to this PR?

/area CI

What this PR does / why we need it:

This PR drops centos7 from our CI by instead relying on zig compiler to provide glibc 2.17 compatible builds for us.

Which issue(s) this PR fixes:

Fixes #3270

Special notes for your reviewer:

Linked libs PRs:

update(cmake): updated c-ares to latest 1.33.1 version. libs#2034 -> new c-ares builds with zig
chore(userspace/libsinsp): only link libanl if present. libs#2036 -> libanl is not present under zig; fixes build
cleanup(userspace/libscap): avoid the usage of non-portable (glibc specific) __gnu_cxx::stdio_filebuf libs#2037 -> title says it all
chore(cmake): honor CMAKE_BUILD_TYPE while building bundled grpc and protobuf libs#2043 -> avoids Falco linking issue undefined symbol: google::protobuf::internal::InternalMetadata::~InternalMetadata()
fix(userspace/libsinsp): fixed a couple of UBs libs#2045 -> fixes arm64 Trace/breakpoint trap issue for threadinfo and k8saudit plugin opening (source_plugin.c UB)
fix(cmake): added zig workarounds for libelf and grpc cmake modules. libs#2049 -> cmake patches for libelf and grpc to support zig build
fix(userspace/libsinsp): avoid possible UB when calling back or front without checking string emptiness libs#2088 -> more UBs fixes for libs

Linked upstream PR:

lib: guard definition of strl{cat,cpy} against glibc version when _FORTIFY_SOURCE is enabled ziglang/zig#21253

TODO:

sanitizer build fails at cmake configure step, because of: zig cc -fsanatize=address fails at link time on macOS ziglang/zig#11403. Workaround: disable zig for sanitizer builds, ie: only use it for to-be-shipped artifacts.
arm64 build is failing while building abseil randen_hwaes.cc file, since their cmake tries to detect platform features and enforces -march=armv8-a+crypto, but armv8 is unsupported by zig. Workaround: patch https://github.com/abseil/abseil-cpp/blob/1031858eb2b87a5d47f236d8eac11d8b05c535d5/absl/copts/GENERATED_AbseilCopts.cmake#L225 to use a supported arch name.
fix test-dev-packages-arm64 -> it seems like a Trace/breakpoint trap is caught during sinsp::open... methods calls:

Tue Sep  3 10:46:35 2024: Loaded event sources: syscall
Tue Sep  3 10:46:35 2024: Enabled event sources: syscall
Tue Sep  3 10:46:35 2024: Opening 'syscall' source with Kernel module
Trace/breakpoint trap

fix test-dev-packages-arm64 -> it seems like a Trace/breakpoint trap is caught related to stats_writer line: output_fields["falco.host_boot_ts"] = machine_info->boot_ts_epoch; -> bca6f31
fix test-dev-packages-arm64 -> it seems like a Trace/breakpoint trap is caught related to k8saudit opening
libelf is built bundled

Does this PR introduce a user-facing change?:

new(ci): use `zig` compiler instead of relying on centos7.

poiana · 2024-08-28T07:59:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [FedeDP]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

FedeDP · 2024-08-28T08:14:16Z

/milestone TBD

FedeDP · 2024-08-28T10:18:48Z

So, we need to disable the build with shared libelf, because zig statically links all specified libraries; see: ziglang/zig#7094
Indeed the CI complains it cannot find system libelf.
Since this was an 🌶️ topic for the CNCF, i am going to hold this PR; at the same time, i think we should actually find a way to drop centos:7 while maintaining same glibc requirements.

Anyway, even trying with BUNDLED_LIBELF, gives a build error...

/hold

submodules/falcosecurity-rules

FedeDP · 2024-08-28T10:20:25Z

.github/workflows/reusable_build_packages.yaml

          cmake -B build -S . \
              -DCMAKE_BUILD_TYPE=${{ inputs.build_type }} \
              -DUSE_BUNDLED_DEPS=On \
+              -DUSE_BUNDLED_LIBELF=On \


Enabled bundled libelf to build with zig. Of course this is not desiderable.

FedeDP · 2024-08-28T14:39:53Z

Now failing the build on

ld.lld: error: undefined symbol: arc4random_buf

referenced by ares_rand.c:270

That's probably because c-ares does some magic to detect that symbol: https://github.com/c-ares/c-ares/blob/main/CMakeLists.txt#L456; that symbol was added in glibc2.36; most probably it is detecting the symbol even if zig is not actually supporting it since we are targeting a build with glibc-2.17.

FedeDP · 2024-08-29T08:01:18Z

I tried to build c-ares from scratch (ie: without Falco involved at all) with zig and it worked fine with the same exports we are doing in the CI.

FedeDP · 2024-08-29T08:24:53Z

Update: i saw that updating c-ares to 1.33.1 (latest release) fixed the issue. I am going to bump it in libs (if possible, ie: if grpc does not complain :/ )

Bump PR in libs: falcosecurity/libs#2034

I now bumped this one to use head of the libs PR with the updated c-ares. 🤞

FedeDP · 2024-08-29T13:43:14Z

So, i now bumped this branch to use libs chore/zig_build branch HEAD; that is a branch i pushed upstream with all zig related fixes on it:

update(cmake): updated c-ares to latest 1.33.1 version. libs#2034
chore(userspace/libsinsp): only link libanl if present. libs#2036
cleanup(userspace/libscap): avoid the usage of non-portable (glibc specific) __gnu_cxx::stdio_filebuf libs#2037
chore(cmake): honor CMAKE_BUILD_TYPE while building bundled grpc and protobuf libs#2043

On that branch, i was able to build a full BUNDLED_DEPS version of sinsp-example!

Moreover, i found out that CMAKE_{C,CXX}_COMPILER truncates CC and CXX strings at first whitespace, leading with cmake using zig as compiler for both, instead of zig cc and zig c++.
I created 2 small wrapper scripts in CI to workaround this issue:

cat > zig-linux-${{ inputs.arch }}-${ZIG_VERSION}/zig-cc << EOF
#!/bin/bash
zig cc -target ${{ inputs.arch }}-linux-gnu.2.17 "$@"
EOF
chmod +x zig-linux-${{ inputs.arch }}-${ZIG_VERSION}/zig-cc
echo "CC=zig-cc" >> $GITHUB_ENV

// same for zig-c++

FedeDP · 2024-08-30T12:23:10Z

To be able to build Falco, for now, i had to disable _FORTIFY_SOURCE, see my commit message: 0bb6f49

I opened an upstream issue to track the problem: ziglang/zig#21252 and a PR: ziglang/zig#21253

FedeDP · 2024-09-30T12:52:40Z

TODO:

rebase on master and fix code fmt
fix arm64 tests :/ -> this was caused by an UB In libs or Falco, will be fixed once we upgrade libs to the master with patch fix(userspace/libsinsp): avoid possible UB when calling back or front without checking string emptiness libs#2088

github-actions · 2024-09-30T14:59:46Z

This PR may bring feature or behavior changes in the Falco engine and may require the engine version to be bumped.

Please double check userspace/engine/falco_engine_version.h file. See versioning for FALCO_ENGINE_VERSION.

/hold

leogr · 2024-09-30T16:06:47Z

This PR may bring feature or behavior changes in the Falco engine and may require the engine version to be bumped.

Please double check userspace/engine/falco_engine_version.h file. See versioning for FALCO_ENGINE_VERSION.

/hold

False positive

FedeDP · 2024-09-30T16:07:52Z

Yep, tomorrow will rebase on latest libs master that fixes arm64 test-dev-packages and unhold this one :)

Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP · 2024-10-01T07:06:22Z

/unhold
Ready!

FedeDP · 2024-10-01T10:48:50Z

📢 it might happen that master build fails because the cached key (ie: zig version) is no more available and zig moved to a newer master version, thus we cannot find the correct zig version to be downloaded.
In that case, i'll update zig to latest in libs and restart the CI here.

FedeDP · 2024-10-01T14:33:24Z

/hold
until next week.

FedeDP · 2024-10-07T08:17:02Z

I'll keep this on hold until we release Falco 0.39.1 to avoid issues with the new CI.

FedeDP · 2024-10-09T14:25:07Z

/unhold
Let's see if master CI complains (i guess so :D )

poiana added do-not-merge/work-in-progress release-note dco-signoff: yes kind/cleanup area/ci labels Aug 28, 2024

poiana requested review from Andreagit97 and Kaizhe August 28, 2024 07:59

poiana added approved size/M labels Aug 28, 2024

poiana added this to the TBD milestone Aug 28, 2024

poiana added the do-not-merge/hold label Aug 28, 2024

FedeDP commented Aug 28, 2024

View reviewed changes

submodules/falcosecurity-rules Outdated Show resolved Hide resolved

FedeDP commented Aug 28, 2024

View reviewed changes

FedeDP force-pushed the new/use_zig_drop_centos7 branch 2 times, most recently from e6e593c to 8925c1f Compare August 28, 2024 14:38

This was referenced Aug 29, 2024

update(cmake): updated c-ares to latest 1.33.1 version. falcosecurity/libs#2034

Merged

cleanup(userspace/libscap): avoid the usage of non-portable (glibc specific) __gnu_cxx::stdio_filebuf falcosecurity/libs#2037

Merged

FedeDP force-pushed the new/use_zig_drop_centos7 branch from 70e175a to 73b8ed5 Compare August 29, 2024 13:49

FedeDP force-pushed the new/use_zig_drop_centos7 branch 4 times, most recently from 903ec35 to 1bf6742 Compare August 30, 2024 14:26

FedeDP force-pushed the new/use_zig_drop_centos7 branch from 850a25e to 5851b6a Compare September 30, 2024 08:45

poiana added size/XL and removed size/M labels Sep 30, 2024

leogr previously approved these changes Sep 30, 2024

View reviewed changes

FedeDP dismissed leogr’s stale review via e27cab7 September 30, 2024 12:50

FedeDP force-pushed the new/use_zig_drop_centos7 branch from 5851b6a to e27cab7 Compare September 30, 2024 12:50

poiana added size/M and removed size/XL labels Sep 30, 2024

FedeDP force-pushed the new/use_zig_drop_centos7 branch from e27cab7 to affc009 Compare September 30, 2024 12:52

FedeDP added 2 commits October 1, 2024 08:33

new(ci): use zig compiler instead of relying on centos7.

baa9505

Signed-off-by: Federico Di Pierro <[email protected]>

chore(userspace/engine): fix build warning.

be9c09d

Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP force-pushed the new/use_zig_drop_centos7 branch from 98d8dfa to be9c09d Compare October 1, 2024 06:33

poiana removed the do-not-merge/hold label Oct 1, 2024

leogr approved these changes Oct 1, 2024

View reviewed changes

poiana added the do-not-merge/hold label Oct 1, 2024

jasondellaluce approved these changes Oct 7, 2024

View reviewed changes

poiana assigned jasondellaluce Oct 7, 2024

poiana removed the do-not-merge/hold label Oct 9, 2024

poiana merged commit c55adf3 into master Oct 9, 2024
36 checks passed

poiana deleted the new/use_zig_drop_centos7 branch October 9, 2024 14:26

FedeDP mentioned this pull request Nov 6, 2024

fix(cmake,docker): avoid cpp-httplib requiring brotli. #3400

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new(ci): use `zig` compiler instead of relying on centos7. #3307

new(ci): use `zig` compiler instead of relying on centos7. #3307

FedeDP commented Aug 28, 2024 •

edited

Loading

poiana commented Aug 28, 2024

FedeDP commented Aug 28, 2024

FedeDP commented Aug 28, 2024

FedeDP Aug 28, 2024

FedeDP commented Aug 28, 2024

FedeDP commented Aug 29, 2024

FedeDP commented Aug 29, 2024 •

edited

Loading

FedeDP commented Aug 29, 2024 •

edited

Loading

FedeDP commented Aug 30, 2024 •

edited

Loading

FedeDP commented Sep 30, 2024 •

edited

Loading

github-actions bot commented Sep 30, 2024

leogr commented Sep 30, 2024

FedeDP commented Sep 30, 2024

FedeDP commented Oct 1, 2024

FedeDP commented Oct 1, 2024

FedeDP commented Oct 1, 2024

FedeDP commented Oct 7, 2024

FedeDP commented Oct 9, 2024

new(ci): use zig compiler instead of relying on centos7. #3307

new(ci): use zig compiler instead of relying on centos7. #3307

Conversation

FedeDP commented Aug 28, 2024 • edited Loading

poiana commented Aug 28, 2024

FedeDP commented Aug 28, 2024

FedeDP commented Aug 28, 2024

FedeDP Aug 28, 2024

Choose a reason for hiding this comment

FedeDP commented Aug 28, 2024

FedeDP commented Aug 29, 2024

FedeDP commented Aug 29, 2024 • edited Loading

FedeDP commented Aug 29, 2024 • edited Loading

FedeDP commented Aug 30, 2024 • edited Loading

FedeDP commented Sep 30, 2024 • edited Loading

github-actions bot commented Sep 30, 2024

leogr commented Sep 30, 2024

FedeDP commented Sep 30, 2024

FedeDP commented Oct 1, 2024

FedeDP commented Oct 1, 2024

FedeDP commented Oct 1, 2024

FedeDP commented Oct 7, 2024

FedeDP commented Oct 9, 2024

new(ci): use `zig` compiler instead of relying on centos7. #3307

new(ci): use `zig` compiler instead of relying on centos7. #3307

FedeDP commented Aug 28, 2024 •

edited

Loading

FedeDP commented Aug 29, 2024 •

edited

Loading

FedeDP commented Aug 29, 2024 •

edited

Loading

FedeDP commented Aug 30, 2024 •

edited

Loading

FedeDP commented Sep 30, 2024 •

edited

Loading